-
Notifications
You must be signed in to change notification settings - Fork 795
[CI] Fix detecting usage of private emails #19394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
5bb431a
to
178d1e6
Compare
|
1c31737
to
93b0594
Compare
PS:- the "private email" workflow is failing in the precommit because I've made my email hidden in the GH profile. To demonstrate the changes made in this PR> |
… Github UI (#148694) **Problem** Currently, the email check workflow uses `git` to see email used for the last commit but the email address used when merging is actually governed by GitHub settings not what's stored in `git`. Due to this, the email check workflow passes even when the author's email is private in Github. We saw several such cases in our fork of llvm. See #17675 **Solution** Try to find user's email using GH's GraphQL APIs. User's email will be null if it's hidden in the profile. --------- Signed-off-by: Agarwal, Udit <[email protected]>
93b0594
to
df8156a
Compare
@@ -39,6 +55,9 @@ jobs: | |||
[{"body" : "$COMMENT"}] | |||
EOF | |||
|
|||
# Fail this job. | |||
false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did we have this part upstream? or they didnt want it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's not a part of upstream as of now. But, as per llvm/llvm-project#148694 (comment), I'll be making a separate PR to make the workflow fail in the upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, thanks
@sarnex I've re-worked the fix based on the feedback I got from the community. The fix in this PR now cherry-picks llvm/llvm-project#148694 and makes the workflow fail when private email is used. I'm working on a separate PR to make the workflow fail in upstream (based on llvm/llvm-project#148694 (comment)), but I suppose that will take some time, so in the meanwhile we can have the fix in intel/llvm atleast. |
This reverts commit efe5a5b.
Reverts #19394 sadly. The workflow is failing if user's email is not listed publicly on your GH profile. This is different from not having your email public on Github (in Github email settings page vs. email field in Github profile/email settings). Reverting this PR till I figure out a fix.
fixes #17675
Currently, in the workflow, we only check the email that was used for the latest commit in the PR. This email can differ from the email settings on the GH UI, from which PR will be authored, once we merge it.